bug: handle git changes outside the workspace root#14
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
|
There was a problem hiding this comment.
Pull request overview
Updates the analyzer to ignore git-changed files that are outside the workspace root, preventing unrelated repo changes from triggering builds.
Changes:
- Ignore changed files that cannot be made relative to the workspace root.
- Add a parametrized test covering changed files outside the workspace.
- Update
tydev dependency and add atyignore for a Hypothesis decorator.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
bough/analyzer.py |
Skips changed files outside the workspace when computing directly affected packages. |
tests/test_complex_scenarios.py |
Adds coverage to ensure out-of-workspace changes produce no affected builds. |
tests/test_formatters.py |
Adds a ty ignore comment for type-checking on Hypothesis decorator usage. |
pyproject.toml |
Updates the ty dev dependency version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bough/analyzer.py
Outdated
| except ValueError as e: | ||
| if "is not in the subpath" in str(e): | ||
| logger.debug("Ignoring file {file_path} outside of workspace") | ||
| continue |
There was a problem hiding this comment.
Path(...).relative_to(...) raises ValueError for “not a subpath”, but this code only continues when the exception message matches a specific substring. If the message differs (Python version variation, different ValueError text), execution falls through with file_path_obj undefined, which can cause an UnboundLocalError later. Recommend avoiding message matching and branching explicitly (e.g., Path(file_path).is_relative_to(strip_prefix)), or catching ValueError and unconditionally skipping the file as “outside workspace” in this context.
| except ValueError as e: | |
| if "is not in the subpath" in str(e): | |
| logger.debug("Ignoring file {file_path} outside of workspace") | |
| continue | |
| except ValueError: | |
| logger.debug(f"Ignoring file {file_path} outside of workspace") | |
| continue |
bough/analyzer.py
Outdated
| file_path_obj = Path(file_path).relative_to(strip_prefix) | ||
| except ValueError as e: | ||
| if "is not in the subpath" in str(e): | ||
| logger.debug("Ignoring file {file_path} outside of workspace") |
There was a problem hiding this comment.
This log line uses {file_path} but isn’t an f-string (and doesn’t use %s interpolation), so it will log the literal braces instead of the actual path. Use an f-string or logger-style parameterized formatting so the file path is included.
| logger.debug("Ignoring file {file_path} outside of workspace") | |
| logger.debug("Ignoring file %s outside of workspace", file_path) |
| config_path = tmp_path / ".bough.yml" | ||
| config_path.write_text(""" | ||
| buildable: | ||
| - "tools/*" | ||
| """) |
There was a problem hiding this comment.
write_text("""...""") includes a leading newline and relies on indentation/formatting of a triple-quoted string for YAML. Consider writing the YAML without the leading newline (or using textwrap.dedent(...).lstrip()), which makes the test fixture less fragile and easier to read.
Closes: PLAT-231